Skip to content

fix: methods returning Promise<RpcStub<T>> no longer produce stub-of-stub result types - #253

Merged
ndisidore merged 2 commits into
mainfrom
fix/stub-brand-leak
Aug 19, 2026
Merged

fix: methods returning Promise<RpcStub<T>> no longer produce stub-of-stub result types#253
ndisidore merged 2 commits into
mainfrom
fix/stub-brand-leak

Conversation

@ndisidore

@ndisidore ndisidore commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Presently declaring an RPC method to return Promise<RpcStub<T>> gave you a broken type: an internal brand property made the type machinery treat the already-stubbed result as something that still needed wrapping, so awaiting yielded RpcStub<RpcStub<T>> (TS2322) and passing the result into another RPC call failed to compile (TS2345), even though the runtime behaved correctly.

Now the type machinery recognizes existing stubs before re-wrapping, so a Promise<RpcStub<T>> return types identically to a Promise<T> return and awaits, pipelined calls, and .map() over stub arrays all compile.

Nothing changes at runtime; the one migration is rewriting any explicit RpcPromise<RpcStub<T>> annotation as RpcPromise<T>.

…ult types

The string-keyed `__RPC_TARGET_BRAND` (and the workers brands in
capnweb-validate) leaked into the keyof-based Provider/ValidatedStub key
mapping, so a stub of a branded target still matched `Stubable`
structurally. A method declared as `Promise<RpcStub<T>>` therefore had
its already-stubbed result stubified again, producing a broken
stub-of-stub type: awaits yielded `RpcStub<RpcStub<T>>` (TS2322) and
passing the result as a pipelined argument failed (TS2345).

Excluding the brand keys from stub surfaces fixes the root cause; on top
of that, `Result` now elides declared stub returns via `ElideStub`, so
`Promise<RpcStub<T>>` returns produce exactly the same `RpcPromise<T>`
as `Promise<T>` returns. Elision is the chosen policy for stubable
payloads only: such stubs await back to a stub either way, so the two
declarations are genuinely interchangeable. Plain-interface stubs
(`RpcStub<PlainApi>`) and `RpcStub<any>` are deliberately NOT elided —
their awaited type would change if they were.

The workerd interop tests gain `<any>` casts where a userspace stub is
handed to workers-types' native `RpcStub`: without the leaked brand, a
userspace stub no longer statically matches workers-types' `Stubable`
(runtime interop is unchanged).
Restoring `__RPC_TARGET_BRAND` in the Provider key mapping keeps
userspace stubs structurally assignable to workers-types' `Stubable`,
so `new RpcStub(userspaceStub)` from `cloudflare:workers` typechecks
again without casts (as it did before the brand exclusion).

The brand exclusion turns out to be unnecessary for the stub-of-stub
fix: `Stubify`/`Result`/`ElideStub` all check `StubBase` before
`Stubable`, so a stub matching `Stubable` is caught as a stub first and
never re-stubified. That ordering is the same mechanism that already
protects callable stubs, which genuinely match `Stubable` and cannot
have that excluded.

Known residual: an explicit `RpcPromise<RpcStub<T>>` annotation still
produces the old double-stub shape (matching main); the changeset
already directs users to write `RpcPromise<T>` instead.
@changeset-bot

changeset-bot Bot commented Aug 19, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: b86d6bb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
capnweb Minor
capnweb-validate Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@ask-bonk

ask-bonk Bot commented Aug 19, 2026

Copy link
Copy Markdown

github run

@pkg-pr-new

pkg-pr-new Bot commented Aug 19, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/capnweb@253

commit: b86d6bb

@ask-bonk

ask-bonk Bot commented Aug 19, 2026

Copy link
Copy Markdown

@ndisidore Bonk workflow failed. Check the logs for details.

View workflow run · To retry, trigger Bonk again.

@ndisidore ndisidore changed the title fix: exclude RPC brands from stub surfaces and elide stubs in RPC res… fix: methods returning Promise<RpcStub<T>> no longer produce stub-of-stub result types Aug 19, 2026

@dimitropoulos dimitropoulos left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type level tests, woop woop

@ndisidore
ndisidore merged commit 46de5a7 into main Aug 19, 2026
8 of 9 checks passed
@github-actions github-actions Bot mentioned this pull request Aug 19, 2026
ndisidore added a commit that referenced this pull request Aug 20, 2026
Apply ElideStub -- the same transformation Result applies to a declared
stub return -- to the RpcPromise constructor signature, so constructing
from a promised stub produces exactly the type a method returning that
stub would. The parameter accepts Promise<PayloadOrStub<T>>: the payload
itself or, for stubable payloads, a stub of it. NoInfer keeps the stub
arm out of inference, so an inferred T is always the promise's own
resolution type; stubs of non-stubable payloads are rejected because
ElideStub wouldn't elide them.

Type tests pin constructor/method equivalence for bare, callable,
plain-interface, and union payloads.

(Extracted from the pre-squash feat/result-stub-elision branch: the
constructor-alignment portion of its history that PR #253 didn't
include.)
ndisidore added a commit that referenced this pull request Aug 20, 2026
* fix: align RpcPromise construction with result elision

Apply ElideStub -- the same transformation Result applies to a declared
stub return -- to the RpcPromise constructor signature, so constructing
from a promised stub produces exactly the type a method returning that
stub would. The parameter accepts Promise<PayloadOrStub<T>>: the payload
itself or, for stubable payloads, a stub of it. NoInfer keeps the stub
arm out of inference, so an inferred T is always the promise's own
resolution type; stubs of non-stubable payloads are rejected because
ElideStub wouldn't elide them.

Type tests pin constructor/method equivalence for bare, callable,
plain-interface, and union payloads.

(Extracted from the pre-squash feat/result-stub-elision branch: the
constructor-alignment portion of its history that PR #253 didn't
include.)

* fix: infer inline object literals with methods in the RpcPromise constructor

new RpcPromise(Promise.resolve({ x: 1, f() { return 1 } })) failed to
type-check: a literal with a method is context-sensitive, and the stub
arm of the constructor's Promise<PayloadOrStub<T>> parameter poisons
the contextual type, collapsing Promise.resolve's inference to never.
The same value predeclared in a const, or with an explicit type
argument, worked fine.

The constructor type is now an overload pair. The first overload takes
a plain Promise<T> -- no stub arm in the contextual type -- which is
the one context-sensitive arguments are typed against. Since
PayloadOrStub's stub arm is NoInfer anyway, both overloads infer
identically; the second (the previous signature) matters only when T
is explicitly annotated and the payload is a stub.

Type tests cover the inline literal, the predeclared equivalent,
explicit-type-argument-with-stub-payload, and a promise for a
target-or-stub union.

* chore: trim redundant type-test coverage and add the constructor changeset

- Drop the explicit-type-argument equivalence case from stub-elision
  (rpc-base-cases pins the same overload with a strict expectType) and
  the consumeMaybe(constructedMaybe) call already implied by the strict
  Equal against maybeViaMethod.
- Add a changeset for the constructor typing change; changeset-bot was
  flagging the PR as bump-less.

* test: pin RpcPromise constructor elision for ValidatedStub payloads

A ValidatedStub payload elides in the constructor (branded targets) or
keeps its stub type (plain interfaces) only because ValidatedStub
structurally matches capnweb's StubBase, which ElideStub keys on --
capnweb-validate maintains its own copy of that shape. Pin both cases
so drift in either package can't silently change the constructor's
type.
dimitropoulos added a commit that referenced this pull request Aug 21, 2026
0.12.0 added a way to construct an `RpcPromise` from an ordinary `Promise`, so
callers can pipeline against a capability you have not obtained yet. That is
new public API, not a bug fix, and the docs said nothing about it -- main
documented it in the root README, which this branch replaced with a pointer to
here, so the merge would otherwise have dropped it on the floor.

`concepts/promises.md` gets a section, and it leans on the equivalence the API
docs make: wrapping a promise means the same thing as a local-loopback call
returning it. That is worth stating plainly, because it derives all the rules
that would otherwise have to be listed as a second set to memorise -- the
resolution is serialized, targets and functions come back as stubs, ownership
transfers, rejections propagate. The ownership one is the sharp edge and is
called out: disposing the promise disposes the resolution, so resolve with a
`.dup()` if you want to keep a stub.

The cheat sheet gets a one-line constructor note, matching the one `RpcStub`
already has.

Also: a Changelog entry in the sidebar under Reference, pointing at the GitHub
releases page. Off-site on purpose -- release notes are generated from
changesets on every publish, so a page here would be a copy that goes stale the
next time anyone ships. Nimbus recognises the absolute URL and adds
`target="_blank" rel="noopener"` itself.

Nothing else in the merge needed documenting. #241, #243, #251 and #253 are
leak and typing fixes with no API surface to describe; #253's note that
`RpcPromise<RpcStub<T>>` should now be written `RpcPromise<T>` applies to no
annotation anywhere in these pages.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants